-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENH] BEP022 - Magnetic Resonance Spectroscopy #425
Conversation
@markmikkelsen one of them has an excellent README but the 2 others have none There is a README template in the bids starter kit if that can help: https://github.com/bids-standard/bids-starter-kit/blob/main/templates/README.MD?plain=1 |
@effigies do you think we should / could use this PR to use citation.cff files? I think we do not have any example that make use of them. |
Not unless Mark already has CITATION.cff files he wants to include. We don't need to pile on tasks to this BEP. Let's make a separate PR that adds a citation file to an existing example. |
oh yeah of course!! that's fair! will open an issue to see if we can actually do it in another dataset that already exist. |
🤦🏾 👴🏾 |
Yep! I've just added these files. |
This pull request is nearly ready for merging. The check failures relate to the MRS filename suffices not being included in the |
- Change suffix from `ref` to `mrsref` where applicable
- Amend dataset_listing.tsv and README.md
There are no examples of |
I'm ignorant of how the validator works, but does it rely on examples? My impression was that it was contingent on the current stable specification. So, how do example datasets factor into validation? |
in short and in a very hand wavy way: the validator tries to implement the specification so we need examples to ensure that the validator work as expected (they act as some sort of ground truth) so the richer the examples, the more we can be sure that the validator has fewer blind spots |
Gotcha. Since the datasets are not real. I can just create a bunch of example of fake datasets to satisfy this. |
for MRSI in mrs_{2dmrsi,biggaba}/sub-*/mrs/*.json; do jq '.SpectrometerFrequency = [.SpectrometerFrequency] | .ResonantNucleus = [.ResonantNucleus]' $MRSI \ | sponge $MRSI; done npx prettier -w mrs_2dmrsi/sub-*/mrs/*.json npx prettier --tab-width 4 -w mrs_biggabba/sub-*/mrs/*.json
@markmikkelsen @wtclarke Note that BIDS doesn't permit null values, but instead leaves fields blank. Fixed in bc92a4d. Not sure if you need to update converters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Remi-Gau Would you mind a final review, since I've been pushing to this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could not spot anything
This pull request adds MRS BIDS-compliant datasets to the
bids-examples
repository. It includes three datasets:mrs_2dmrsi/
,mrs_biggaba/
, andmrs_fmrs/
.